Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Infrastructure Dependency on Web #54

Closed

Conversation

sdepouw
Copy link
Collaborator

@sdepouw sdepouw commented Feb 21, 2019

  • Fixes Infrastructure Registry #53
  • CleanArchitecture.Web no longer has a dependency on CleanArchitecture.Infrastructure
    • The DLL is copied to the Web's output/bin directory via a post-build command.
  • The Infrastructure project is now referenced by loading the assembly in Startup.cs, by loading the physical file
    • Loading the assembly via Assembly.Load() does not appear to work, since the Web project does not directly reference it
    • AutoFac does not support scanning assemblies for Modules (the AutoFac equivalent of StructureMap's Registry classes) using string assembly names
  • All Infrastructure convention dependencies are still registered via RegisterAssemblyTypes
  • AppDbContext is registered via DatabaseModule, which is referenced in Startup.cs via builder.RegisterAssemblyModules.

@ardalis
Copy link
Owner

ardalis commented Feb 22, 2019

Build is failing, probably because it's running on linux not Windows and the build script is using the windows copy command?

/usr/share/dotnet/sdk/2.2.103/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.DefaultItems.targets(153,5): warning NETSDK1071: A PackageReference to 'Microsoft.AspNetCore.App' specified a Version of `2.1.5`. Specifying the version of this package is not recommended. For more information, see https://aka.ms/sdkimplicitrefs [/home/vsts/work/1/s/src/CleanArchitecture.Infrastructure/CleanArchitecture.Infrastructure.csproj]
/usr/share/dotnet/sdk/2.2.103/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.DefaultItems.targets(153,5): warning NETSDK1071: A PackageReference to 'Microsoft.AspNetCore.App' specified a Version of `2.1.5`. Specifying the version of this package is not recommended. For more information, see https://aka.ms/sdkimplicitrefs [/home/vsts/work/1/s/src/CleanArchitecture.Web/CleanArchitecture.Web.csproj]
/home/vsts/work/1/s/src/CleanArchitecture.Infrastructure/CleanArchitecture.Infrastructure.csproj(22,5): error MSB3073: The command "copy /Y "/home/vsts/work/1/s/src/CleanArchitecture.Infrastructure/bin/Release/netcoreapp2.1/CleanArchitecture.Infrastructure.dll" "/home/vsts/work/1/s/src\CleanArchitecture.Web\bin/Release/netcoreapp2.1//CleanArchitecture.Infrastructure.dll"" exited with code 127.
    2 Warning(s)
    1 Error(s)

@sdepouw
Copy link
Collaborator Author

sdepouw commented Feb 22, 2019

@ardalis yeah looks like it. Digging into the build log I found:

/bin/sh: 2: /tmp/tmp87e8284eeabb412ead5c221728fcb344.exec.cmd: copy: not found

And after Infrastructure builds we indeed call that post-build event:

copy /Y "$(TargetDir)$(TargetName).dll" "$(SolutionDir)src\CleanArchitecture.Web\$(OutDir)\$(TargetName).dll"

It looks like the post-build step to manually copy the Infrastructure DLL to the output directory needs to be updated to a universal copy command of some kind.

@sdepouw
Copy link
Collaborator Author

sdepouw commented Feb 22, 2019

@ardalis we could also update the build to run against vmImage: 'VS2017-Win2016' instead of vmImage: 'Ubuntu-16.04', unless we're running the Azure DevOps build against Linux for a specific reason?

Copy link
Contributor

@Xeinaemm Xeinaemm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder that its appropriate way of thinking about removing infrastructure reference.

We can have two different approaches to building an app with this architecture.

First, self-hosted REST API + self-hosted SPA, that definitely remove reference to the infrastructure and keep all necessary references in place without hacks.

Second, both in bulk as now, but what about the situation when you will add some logic to infrastructure that belongs to configuration or helpers(with 3rd part references) and you need to use them in Web API?
I have this problem because I've created helpers for a data shaping mechanism, HATEOAS that lives inside infrastructure due to 3rd part references and used in both, infrastructure and web.

You can create an adapter to your own facade that hides implementation under core's service interfaces, but this overhead to helpers is appropriate?

As we know, developers are lazy and creating an abstraction over common mechanisms like extensions will be too much for some people.

I don't really see that web should have any additional logic despite display a result or create endpoints because we will just copy and paste this same 3rd part references and it can create friction because it's used in two places(infrastructure and web).

On the other hand, it completely hides all logic under services or repositories, you cannot force to break that and I agree that is the best way of thinking but with some boilerplate code.

options.UseSqlite("Data Source=database.sqlite")); // will be created in web project root
// options.UseInMemoryDatabase(dbName));
//options.UseSqlServer(Configuration.GetConnectionString("DefaultConnection")));
// TODO: Not sure where to call this database init now that we don't have access to AppDbContext
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What to do for lack of access to the AppDbContext type?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use reflection to grab the infra assembly and instantiate it from there?

Scott DePouw added 3 commits August 22, 2019 08:47
…there's access to AppDbContext. Add ref to Abstractions Microsoft NuGet package in Core. Add Ardalis EFCore package to Web, so it's available at runtime where it's needed.
@ardalis
Copy link
Owner

ardalis commented Nov 8, 2019

@sdepouw Can we get this updated or abandon it?

@ardalis
Copy link
Owner

ardalis commented Apr 24, 2020

@sdepouw Any update on this? Trying to clean up old PRs.

@TheAtomicOption
Copy link

TheAtomicOption commented Feb 24, 2021

In a recent project I did a much more simplistic load of an Infrastructure assembly to remove the Web project reference to it. (Since connection strings are static per instance of my app, I pushed them from Web's Configuration into a static property of a class in Infrastructure, using an Interface from Shared. With that static variable set, Infrastructure can use it to keep awareness/management of db connections completely contained to itself.)

From that experience, one issue here may be that Reflection's Assembly.LoadFile(..) loads the file with no load context. To load with context change to Assembly.Load(..), using just the assembly name without the file extension or path as the parameter. This will load the assembly by looking through the normal priority list of locations for it, and then loading it into the current context so references to types and static variables match the ones referenced later in assemblies like Core.

(see: https://docs.microsoft.com/en-us/dotnet/framework/deployment/best-practices-for-assembly-loading?redirectedfrom=MSDN#understand-the-advantages-and-disadvantages-of-load-contexts)

@ardalis
Copy link
Owner

ardalis commented Apr 2, 2021

Closing this as it's pretty stale. Maybe I'll write a blog post with a smaller repo to demo how to do this.

@ardalis ardalis closed this Apr 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Infrastructure Registry
4 participants